server/streamable_http.go (5)

424-429: Guard http.Flusher access

w.(http.Flusher) will panic if the underlying writer does not implement Flusher (e.g. certain test doubles). Always assert the capability first.

- w.(http.Flusher).Flush()
+ if flusher, ok := w.(http.Flusher); ok {
+     flusher.Flush()
+ }

---

508-555: Notification fan-out may silently drop messages

eventChan is buffered with capacity 10. When the buffer is full the fallback is to block; however the select defaults to returning on <-notifDone>.
If the client is slow, notifications will be lost without feedback.
Consider:

• Using a larger buffer or an unbounded queue.
• Back-pressure mechanisms (e.g. context cancellation).
• Logging or propagating the overflow error.

---

748-759: event argument is unused in callers – clarify or remove

writeSSEEvent allows specifying an event: field but nothing in this file invokes it with a non-empty value. If custom event names are required, document and use them; otherwise drop the parameter to avoid confusion.

---

323-329: Errors from notification handling are discarded

server.HandleMessage may return an error for notifications (e.g. malformed data). Returning 202 Accepted unconditionally can hide failures.
At minimum, log the error and return 400/500 when appropriate.

---

829-845: Dead code – validateSession is never used

The helper looks correct but is not referenced anywhere. Remove it or wire it into handlePost/handleGet to ensure consistent validation.

examples/minimal_client/main.go (2)

24-26: Shared context may cancel long-running requests

Both the initialize and tools/call requests reuse the same 30 s context. If the first call consumes most of the timeout, the second may fail prematurely. Prefer deriving a fresh context per operation.

initCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
...
callCtx, cancel := context.WithTimeout(ctx, 30*time.Second)

---

21-22: Ignore Close error

trans.Close() returns an error (according to transport interface) but it is discarded. Consider logging it to aid debugging of lingering connections.

examples/minimal_server/main.go (1)

64-69: Handle http.ErrServerClosed gracefully

When Shutdown is invoked the listener returns http.ErrServerClosed, which is expected and should not be treated as fatal.

-if err := streamableServer.Start(":8080"); err != nil {
-    log.Fatalf("Failed to start server: %v", err)
-}
+if err := streamableServer.Start(":8080"); err != nil && err != http.ErrServerClosed {
+    log.Fatalf("Failed to start server: %v", err)
+}

examples/streamable_http_client/main.go (1)

1-56: Well-structured example with clear flow and good practices

This example demonstrates a streamable HTTP client with all essential components: transport creation, notification handling, session initialization, and response display. The code follows Go best practices with proper error handling and resource cleanup.

Consider adding more detailed comments explaining the expected behavior and outputs at key points, especially for the notification handler. For instance, explaining what kind of notifications might be received and how they're structured would help users understand the example better.

examples/streamable_http_client_complete/main.go (1)

1-131: Comprehensive client example with complete MCP workflow

This example builds on the basic client by adding tool listing, tool invocation, and notification handling with signal management. The structure is logical and demonstrates a complete interaction flow with an MCP server.

Consider adding comments explaining the expected notification content from the echo tool, particularly in the notification handler setup (lines 27-31) or near the waiting section (lines 116-119). This would help users understand what to expect when running the example.

examples/streamable_http_server/main.go (1)

61-66: Consider adding context check in notification goroutine

The notification goroutine doesn't check if the context is still valid before sending the notification, which could lead to errors if the server is shutting down.

go func() {
	time.Sleep(1 * time.Second)
+	// Check if context is still valid before sending notification
+	select {
+	case <-ctx.Done():
+		return
+	default:
		mcpServer.SendNotificationToClient(ctx, "echo/notification", map[string]interface{}{
			"message": "Echo notification: " + message,
		})
+	}
}()

README-streamable-http.md (3)

22-25: Fix bullet point formatting

The bullet points in this section have loose punctuation marks that should be fixed for better readability.

### Key Components

- `StreamableHTTPServer`: The main server implementation that handles HTTP requests and responses
- `streamableHTTPSession`: Represents an active session with a client
- `EventStore`: Interface for storing and retrieving events for resumability
- `InMemoryEventStore`: A simple in-memory implementation of the EventStore interface

🧰 Tools

🪛 LanguageTool

[uncategorized] ~22-~22: Loose punctuation mark.
Context: ...Key Components - StreamableHTTPServer: The main server implementation that han...

(UNLIKELY_OPENING_PUNCTUATION)

---

34-37: Consider expanding the client implementation section

The client implementation section is quite brief compared to the server section. Adding more details about the client's key components, options, and design considerations would make the documentation more balanced.

Consider expanding this section to include:

• Key client components
• Client options (similar to server options section)
• Design considerations specific to the client implementation

---

273-302: Add a troubleshooting section

The documentation would benefit from a troubleshooting section that addresses common issues users might encounter, such as connection problems, session expiration, or event replay failures.

Consider adding a "Troubleshooting" section that covers:

• Common error scenarios and their solutions
• Debugging tips (e.g., enabling verbose logging)
• Best practices for error handling in both client and server implementations

server/streamable_http_test.go (3)

73-92: Consider flattening nested conditionals for better readability

The nested conditionals for validating the response structure could be simplified for better readability and easier debugging when tests fail.

- if result, ok := response["result"].(map[string]interface{}); ok {
-   if serverInfo, ok := result["serverInfo"].(map[string]interface{}); ok {
-     if serverInfo["name"] != "test-server" {
-       t.Errorf("Expected server name test-server, got %v", serverInfo["name"])
-     }
-     if serverInfo["version"] != "1.0.0" {
-       t.Errorf("Expected server version 1.0.0, got %v", serverInfo["version"])
-     }
-   } else {
-     t.Errorf("Expected serverInfo in result, got none")
-   }
- } else {
-   t.Errorf("Expected result in response, got none")
- }
+ result, ok := response["result"].(map[string]interface{})
+ if !ok {
+   t.Fatalf("Expected result in response, got none")
+ }
+ 
+ serverInfo, ok := result["serverInfo"].(map[string]interface{})
+ if !ok {
+   t.Fatalf("Expected serverInfo in result, got none")
+ }
+ 
+ if serverInfo["name"] != "test-server" {
+   t.Errorf("Expected server name test-server, got %v", serverInfo["name"])
+ }
+ 
+ if serverInfo["version"] != "1.0.0" {
+   t.Errorf("Expected server version 1.0.0, got %v", serverInfo["version"])
+ }

---

257-257: Replace hard-coded sleep with a more reliable mechanism

Using a fixed sleep duration can lead to flaky tests on different environments or under different load conditions.

Consider using a more robust synchronization mechanism or at least making the sleep duration configurable:

- // Wait a bit for the stream to be established
- time.Sleep(100 * time.Millisecond)
+ // Wait for the stream to be established
+ streamEstablishmentTimeout := 200 * time.Millisecond
+ time.Sleep(streamEstablishmentTimeout)

For even better reliability, you could implement a ready signal from the server once the stream is established.

---

293-312: Implement more robust SSE event parsing

The current SSE event parsing is simplified and assumes a specific format. A more robust implementation would handle multiple data lines, event IDs, and other SSE features.

Consider implementing a more comprehensive SSE parser or using the same timeout pattern recommended for the previous event reading loop.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

server/streamable_http_test.go (1)

160-178: Add timeout to prevent test hanging.

The event reading loop lacks a timeout mechanism which could cause the test to hang indefinitely if there's an issue with the event stream.

server/streamable_http.go (1)

391-392: ⚠️ Potential issue

Fix redundant check for stateless mode.

This check has the same issue as the previous one - s.sessionIDGenerator() will never return an empty string.

- // Only create a session if we're not in stateless mode
- if s.sessionIDGenerator() != "" {
+ // Only create a session if we're not in stateless mode
+ if !s.stateless {

server/streamable_http_test.go (2)

355-356: Simplify string creation by avoiding unnecessary fmt.Sprintf.

The fmt.Sprintf is unnecessary when the entire string is static.

- rawNotification := fmt.Sprintf(`{"jsonrpc":"2.0","method":"test/notification","params":{"message":"Hello, world!"}}`)
+ rawNotification := `{"jsonrpc":"2.0","method":"test/notification","params":{"message":"Hello, world!"}}`

🧰 Tools

🪛 golangci-lint (1.64.8)

355-355: S1039: unnecessary use of fmt.Sprintf

(gosimple)

---

356-370: Consider removing redundant manual notification handling.

This code creates a fallback notification in case the actual notification doesn't have the expected format, but it's unnecessary if the notification format is consistent. The test should verify what was actually sent rather than creating a second fallback notification.

- // Create a notification with the correct format for testing
- rawNotification := fmt.Sprintf(`{"jsonrpc":"2.0","method":"test/notification","params":{"message":"Hello, world!"}}`)
-
- // Parse the raw notification
- var manualNotification map[string]interface{}
- if err := json.Unmarshal([]byte(rawNotification), &manualNotification); err != nil {
-   t.Fatalf("Failed to decode manual notification: %v", err)
- }
-
- // Check if message exists in params
- message, ok := params["message"]
- if !ok {
-   // If message doesn't exist in params, use the manual notification for testing
-   manualParams := manualNotification["params"].(map[string]interface{})
-   message = manualParams["message"]
-   t.Logf("Using manual notification for testing")
- }
+ // Check if message exists in params
+ message, ok := params["message"]
+ if !ok {
+   t.Errorf("Expected message in params, but not found")
+   return
+ }

server/streamable_http.go (6)

22-22: Remove unused field lastEventID.

The lastEventID field is declared but never used in the code.

type streamableHTTPSession struct {
	sessionID           string
	notificationChannel chan mcp.JSONRPCNotification
	initialized         atomic.Bool
-	lastEventID         string
	eventStore          EventStore
	sessionTools        sync.Map // Maps tool name to ServerTool
}

🧰 Tools

🪛 golangci-lint (1.64.8)

22-22: field lastEventID is unused

(unused)

---

214-214: Remove unused field streamMapping.

The streamMapping field is declared but only used in the unused writeSSEEvent method.

type StreamableHTTPServer struct {
	server             *MCPServer
	baseURL            string
	basePath           string
	endpoint           string
	sessions           sync.Map // Maps sessionID to ClientSession
	srv                *http.Server
	contextFunc        SSEContextFunc
	sessionIDGenerator func() string
	enableJSONResponse bool
	eventStore         EventStore
	standaloneStreamID string
-	streamMapping      sync.Map // Maps streamID to response writer
	requestToStreamMap sync.Map // Maps requestID to streamID
}

🧰 Tools

🪛 golangci-lint (1.64.8)

214-214: field streamMapping is unused

(unused)

---

683-711: Remove unused method writeSSEEvent.

This method is never called in the codebase. It appears to be leftover code from an earlier implementation approach.

- // writeSSEEvent writes an SSE event to the given stream
- func (s *StreamableHTTPServer) writeSSEEvent(streamID string, event string, message mcp.JSONRPCMessage) error {
-	// Get the stream channel
-	streamChanI, ok := s.streamMapping.Load(streamID)
-	if !ok {
-		return fmt.Errorf("stream not found: %s", streamID)
-	}
-
-	streamChan, ok := streamChanI.(chan string)
-	if !ok {
-		return fmt.Errorf("invalid stream channel type")
-	}
-
-	// Marshal the message
-	data, err := json.Marshal(message)
-	if err != nil {
-		return err
-	}
-
-	// Create the event data
-	eventData := fmt.Sprintf("event: %s\ndata: %s\n\n", event, data)
-
-	// Send the event to the channel
-	select {
-	case streamChan <- eventData:
-		return nil
-	default:
-		return fmt.Errorf("stream channel full")
-	}
- }

🧰 Tools

🪛 golangci-lint (1.64.8)

683-683: func (*StreamableHTTPServer).writeSSEEvent is unused

(unused)

---

763-778: Remove unused method validateSession.

This method is defined but never called in the codebase.

- // validateSession checks if the session ID is valid and the session is initialized
- func (s *StreamableHTTPServer) validateSession(sessionID string) bool {
-	// Check if the session ID is valid
-	if sessionID == "" {
-		return false
-	}
-
-	// Check if the session exists
-	if sessionValue, ok := s.sessions.Load(sessionID); ok {
-		// Check if the session is initialized
-		if httpSession, ok := sessionValue.(*streamableHTTPSession); ok {
-			return httpSession.Initialized()
-		}
-	}
-
-	return false
- }

🧰 Tools

🪛 golangci-lint (1.64.8)

763-763: func (*StreamableHTTPServer).validateSession is unused

(unused)

---

496-497: Use structured logging instead of fmt.Printf.

Direct use of fmt.Printf for error logging is not ideal for a library. Consider using a proper logging interface or returning errors to the caller.

- // Log the error but continue
- fmt.Printf("Error replaying events: %v\n", err)
+ // If the server has a logger, use it
+ if s.server.logger != nil {
+     s.server.logger.Printf("Error replaying events: %v", err)
+ }

Apply similar changes to other instances of fmt.Printf throughout the code.

---

621-622: Simplify string creation by avoiding unnecessary fmt.Sprintf.

The fmt.Sprintf is unnecessary when the entire string is static.

- initialEvent := fmt.Sprintf("data: {\"jsonrpc\": \"2.0\", \"method\": \"connection/established\"}\n\n")
+ initialEvent := "data: {\"jsonrpc\": \"2.0\", \"method\": \"connection/established\"}\n\n"

🧰 Tools

🪛 golangci-lint (1.64.8)

621-621: S1039: unnecessary use of fmt.Sprintf

(gosimple)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

server/streamable_http.go (8)

220-223: Remove unused field streamMapping

The streamMapping field is only used in the writeSSEEvent method, which itself is unused according to static analysis.

	standaloneStreamID string
-	streamMapping      sync.Map // Maps streamID to response writer
	requestToStreamMap sync.Map // Maps requestID to streamID
	statelessMode      bool

🧰 Tools

🪛 golangci-lint (1.64.8)

221-221: field streamMapping is unused

(unused)

---

628-629: Simplify string formatting

Use a simple string literal instead of fmt.Sprintf when no formatting is needed.

-	initialEvent := fmt.Sprintf("data: {\"jsonrpc\": \"2.0\", \"method\": \"connection/established\"}\n\n")
+	initialEvent := "data: {\"jsonrpc\": \"2.0\", \"method\": \"connection/established\"}\n\n"

🧰 Tools

🪛 golangci-lint (1.64.8)

628-628: S1039: unnecessary use of fmt.Sprintf

(gosimple)

---

690-718: Remove unused writeSSEEvent method

The writeSSEEvent method is never called in the codebase. Consider removing it unless it's intended for future use.

-// writeSSEEvent writes an SSE event to the given stream
-func (s *StreamableHTTPServer) writeSSEEvent(streamID string, event string, message mcp.JSONRPCMessage) error {
-	// Get the stream channel
-	streamChanI, ok := s.streamMapping.Load(streamID)
-	if !ok {
-		return fmt.Errorf("stream not found: %s", streamID)
-	}
-
-	streamChan, ok := streamChanI.(chan string)
-	if !ok {
-		return fmt.Errorf("invalid stream channel type")
-	}
-
-	// Marshal the message
-	data, err := json.Marshal(message)
-	if err != nil {
-		return err
-	}
-
-	// Create the event data
-	eventData := fmt.Sprintf("event: %s\ndata: %s\n\n", event, data)
-
-	// Send the event to the channel
-	select {
-	case streamChan <- eventData:
-		return nil
-	default:
-		return fmt.Errorf("stream channel full")
-	}
-}

🧰 Tools

🪛 golangci-lint (1.64.8)

690-690: func (*StreamableHTTPServer).writeSSEEvent is unused

(unused)

---

770-785: Remove unused validateSession method

The validateSession method is never called in the codebase. Consider removing it unless it's intended for future use.

-// validateSession checks if the session ID is valid and the session is initialized
-func (s *StreamableHTTPServer) validateSession(sessionID string) bool {
-	// Check if the session ID is valid
-	if sessionID == "" {
-		return false
-	}
-
-	// Check if the session exists
-	if sessionValue, ok := s.sessions.Load(sessionID); ok {
-		// Check if the session is initialized
-		if httpSession, ok := sessionValue.(*streamableHTTPSession); ok {
-			return httpSession.Initialized()
-		}
-	}
-
-	return false
-}

🧰 Tools

🪛 golangci-lint (1.64.8)

770-770: func (*StreamableHTTPServer).validateSession is unused

(unused)

---

22-22: Remove unused field lastEventID

The lastEventID field in streamableHTTPSession is declared but never used.

	notificationChannel chan mcp.JSONRPCNotification
	initialized         atomic.Bool
-	lastEventID         string
	eventStore          EventStore

🧰 Tools

🪛 golangci-lint (1.64.8)

22-22: field lastEventID is unused

(unused)

---

502-504: Add structured logging instead of fmt.Printf

Replace fmt.Printf with a proper logging mechanism for error reporting.

-		// Log the error but continue
-		fmt.Printf("Error replaying events: %v\n", err)
+		// Log the error but continue
+		// Consider using a proper logger like logrus or zap
+		// Or if you prefer to keep it simple:
+		log.Printf("Error replaying events: %v", err)

---

522-523: Add structured logging instead of fmt.Printf

Similar to the previous comment, replace fmt.Printf with a proper logging mechanism.

-		// Log the error but continue
-		fmt.Printf("Error storing event: %v\n", storeErr)
+		// Log the error but continue
+		// Consider using a proper logger like logrus or zap
+		// Or if you prefer to keep it simple:
+		log.Printf("Error storing event: %v", storeErr)

---

559-560: Add structured logging instead of fmt.Printf

Again, replace fmt.Printf with a proper logging mechanism.

-		// Log the error but continue
-		fmt.Printf("Error storing event: %v\n", storeErr)
+		// Log the error but continue
+		// Consider using a proper logger like logrus or zap
+		// Or if you prefer to keep it simple:
+		log.Printf("Error storing event: %v", storeErr)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro